xen/arm64: correctly emulate the {w, x}zr registers

On AArch64, encoding 31 for an R<n> in the HSR is used to represent
either {w,x}sp or {w,x}zr (See C1.2.4 in ARM DDI 0486A.d) depending on
how the register field is interpreted by the instruction.

All the instructions trapped by Xen (either via a sysreg access or
data abort) interpret encoding 31 as {w,x}zr. Therefore we don't have
to worry about the possibility that a trap could refer to sp or about
decoding the instruction.

For example AArch64 LDR and STR can have zr in the source/target
register <Xt>, but never sp. sp can be present in the destination
pointer( i.e.  "[sp]"), but that would be represented by the value of
FAR_EL2, not in the HSR.

For AArch32 it is possible for a LDR to target the PC, but this would
not result in a valid ISS in the HSR register. However this could only
occur if loading or storing the PC to MMIO, which we simply choose not
to support for now.

Finally, features such as xenaccess can lead to us trapping on
arbitrary instructions accessing RAM and not just for MMIO. However in
many such cases HSR.ISS is not valid and in general features such as
xenaccess do not rely on the nature of the specific instruction, they
resolve the fault (via information found elsewhere e.g. FAR_EL2)
without needing to know anything about the instruction which triggered
the trap.

The register zr represents the zero register, i.e it will always
return 0 and write to it is ignored. To properly handle this property,
2 new helpers have been introduced {get,set}_user_reg to read/write a
value from/to a register. All the calls to select_user_reg have been
replaced by these 2 helpers.

Furthermore, the code to emulate encoding 31 in select_user_reg has been
dropped because it was invalid. For Aarch64 context, the encoding is
used for sp or zr. For AArch32 context, the ISS won't be valid for data
abort from AArch32 using r15 (i.e pc) as source/destination (See D7-1881
ARM DDI 0487A.d, note the validity is more restrictive than on ARMv7).
It's also not possible to use r15 in co-processor instructions.

This patch fixes setting MMIO register and sysreg to a random value
(actually PC) instead of zero by something like:

*((volatile int*)reg) = 0;

compilers tend to generate "str wzr, [xx]" here.

[ian: added BUG_ON to select_user_reg and clarified bits of the commit message]
Reported-by: Marc Zyngier <Marc.Zyngier@arm.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 7e299438ee7669ba0fce688de9fda84ce108b539..0156755baf89986cf37811da7aa13f4a41400e77 100644 (file)
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -24,12 +24,19 @@
 #include <asm/mmio.h>
 
 static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
-                       mmio_info_t *info, register_t *r)
+                       mmio_info_t *info)
 {
     const struct hsr_dabt dabt = info->dabt;
+    struct cpu_user_regs *regs = guest_cpu_user_regs();
+    /*
+     * Initialize to zero to avoid leaking data if there is an
+     * implementation error in the emulation (such as not correctly
+     * setting r).
+     */
+    register_t r = 0;
     uint8_t size = (1 << dabt.size) * 8;
 
-    if ( !handler->ops->read(v, info, r, handler->priv) )
+    if ( !handler->ops->read(v, info, &r, handler->priv) )
         return 0;
 
     /*
@@ -37,7 +44,7 @@ static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
      * Note that we expect the read handler to have zeroed the bits
      * outside the requested access size.
      */
-    if ( dabt.sign && (*r & (1UL << (size - 1))) )
+    if ( dabt.sign && (r & (1UL << (size - 1))) )
     {
         /*
          * We are relying on register_t using the same as
@@ -45,21 +52,30 @@ static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
          * code smaller.
          */
         BUILD_BUG_ON(sizeof(register_t) != sizeof(unsigned long));
-        *r |= (~0UL) << size;
+        r |= (~0UL) << size;
     }
 
+    set_user_reg(regs, dabt.reg, r);
+
     return 1;
 }
 
+static int handle_write(const struct mmio_handler *handler, struct vcpu *v,
+                        mmio_info_t *info)
+{
+    const struct hsr_dabt dabt = info->dabt;
+    struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+    return handler->ops->write(v, info, get_user_reg(regs, dabt.reg),
+                               handler->priv);
+}
+
 int handle_mmio(mmio_info_t *info)
 {
     struct vcpu *v = current;
     int i;
     const struct mmio_handler *handler = NULL;
     const struct vmmio *vmmio = &v->domain->arch.vmmio;
-    struct hsr_dabt dabt = info->dabt;
-    struct cpu_user_regs *regs = guest_cpu_user_regs();
-    register_t *r = select_user_reg(regs, dabt.reg);
 
     for ( i = 0; i < vmmio->num_entries; i++ )
     {
@@ -74,9 +90,9 @@ int handle_mmio(mmio_info_t *info)
         return 0;
 
     if ( info->dabt.write )
-        return handler->ops->write(v, info, *r, handler->priv);
+        return handle_write(handler, v, info);
     else
-        return handle_read(handler, v, info, r);
+        return handle_read(handler, v, info);
 }
 
 void register_mmio_handler(struct domain *d,

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 31d21155386b5cee7a4513e02fea1a7bd8171437..1516abd09b1b4dd9c179613ce6174b44ad6f2b3a 100644 (file)
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -149,7 +149,31 @@ static void print_xen_info(void)
            debug_build() ? 'y' : 'n', print_tainted(taint_str));
 }
 
-register_t *select_user_reg(struct cpu_user_regs *regs, int reg)
+#ifdef CONFIG_ARM_32
+static inline bool_t is_zero_register(int reg)
+{
+    /* There is no zero register for ARM32 */
+    return 0;
+}
+#else
+static inline bool_t is_zero_register(int reg)
+{
+    /*
+     * For store/load and sysreg instruction, the encoding 31 always
+     * corresponds to {w,x}zr which is the zero register.
+     */
+    return (reg == 31);
+}
+#endif
+
+/*
+ * Returns a pointer to the given register value in regs, taking the
+ * processor mode (CPSR) into account.
+ *
+ * Note that this function should not be used directly but via
+ * {get,set}_user_reg.
+ */
+static register_t *select_user_reg(struct cpu_user_regs *regs, int reg)
 {
     BUG_ON( !guest_mode(regs) );
 
@@ -207,16 +231,31 @@ register_t *select_user_reg(struct cpu_user_regs *regs, int reg)
     }
 #undef REGOFFS
 #else
-    /* In 64 bit the syndrome register contains the AArch64 register
-     * number even if the trap was from AArch32 mode. Except that
-     * AArch32 R15 (PC) is encoded as 0b11111.
+    /*
+     * On 64-bit the syndrome register contains the register index as
+     * viewed in AArch64 state even if the trap was from AArch32 mode.
      */
-    if ( reg == 0x1f /* && is aarch32 guest */)
-        return &regs->pc;
+    BUG_ON(is_zero_register(reg)); /* Cannot be {w,x}zr */
     return &regs->x0 + reg;
 #endif
 }
 
+register_t get_user_reg(struct cpu_user_regs *regs, int reg)
+{
+    if ( is_zero_register(reg) )
+        return 0;
+
+    return *select_user_reg(regs, reg);
+}
+
+void set_user_reg(struct cpu_user_regs *regs, int reg, register_t value)
+{
+    if ( is_zero_register(reg) )
+        return;
+
+    *select_user_reg(regs, reg) = value;
+}
+
 static const char *decode_fsc(uint32_t fsc, int *level)
 {
     const char *msg = NULL;
@@ -1241,22 +1280,19 @@ static arm_hypercall_t arm_hypercall_table[] = {
 #ifndef NDEBUG
 static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
 {
-    register_t *r;
     uint32_t reg;
     uint32_t domid = current->domain->domain_id;
     switch ( code ) {
     case 0xe0 ... 0xef:
         reg = code - 0xe0;
-        r = select_user_reg(regs, reg);
         printk("DOM%d: R%d = 0x%"PRIregister" at 0x%"PRIvaddr"\n",
-               domid, reg, *r, regs->pc);
+               domid, reg, get_user_reg(regs, reg), regs->pc);
         break;
     case 0xfd:
         printk("DOM%d: Reached %"PRIvaddr"\n", domid, regs->pc);
         break;
     case 0xfe:
-        r = select_user_reg(regs, 0);
-        printk("%c", (char)(*r & 0xff));
+        printk("%c", (char)(get_user_reg(regs, 0) & 0xff));
         break;
     case 0xff:
         printk("DOM%d: DEBUG\n", domid);
@@ -1614,7 +1650,7 @@ static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
 
 /* Read as zero and write ignore */
 static void handle_raz_wi(struct cpu_user_regs *regs,
-                          register_t *reg,
+                          int regidx,
                           bool_t read,
                           const union hsr hsr,
                           int min_el)
@@ -1625,7 +1661,7 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
         return inject_undef_exception(regs, hsr);
 
     if ( read )
-        *reg = 0;
+        set_user_reg(regs, regidx, 0);
     /* else: write ignored */
 
     advance_pc(regs, hsr);
@@ -1633,7 +1669,7 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
 
 /* Write only as write ignore */
 static void handle_wo_wi(struct cpu_user_regs *regs,
-                         register_t *reg,
+                         int regidx,
                          bool_t read,
                          const union hsr hsr,
                          int min_el)
@@ -1652,7 +1688,7 @@ static void handle_wo_wi(struct cpu_user_regs *regs,
 
 /* Read only as read as zero */
 static void handle_ro_raz(struct cpu_user_regs *regs,
-                          register_t *reg,
+                          int regidx,
                           bool_t read,
                           const union hsr hsr,
                           int min_el)
@@ -1666,7 +1702,7 @@ static void handle_ro_raz(struct cpu_user_regs *regs,
         return inject_undef_exception(regs, hsr);
     /* else: raz */
 
-    *reg = 0;
+    set_user_reg(regs, regidx, 0);
 
     advance_pc(regs, hsr);
 }
@@ -1675,7 +1711,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
                        const union hsr hsr)
 {
     const struct hsr_cp32 cp32 = hsr.cp32;
-    register_t *r = select_user_reg(regs, cp32.reg);
+    int regidx = cp32.reg;
     struct vcpu *v = current;
 
     if ( !check_conditional_instr(regs, hsr) )
@@ -1708,7 +1744,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
         if ( psr_mode_is_user(regs) )
             return inject_undef_exception(regs, hsr);
         if ( cp32.read )
-           *r = v->arch.actlr;
+            set_user_reg(regs, regidx, v->arch.actlr);
         break;
 
     /*
@@ -1740,13 +1776,13 @@ static void do_cp15_32(struct cpu_user_regs *regs,
     case HSR_CPREG32(PMUSERENR):
         /* RO at EL0. RAZ/WI at EL1 */
         if ( psr_mode_is_user(regs) )
-            return handle_ro_raz(regs, r, cp32.read, hsr, 0);
+            return handle_ro_raz(regs, regidx, cp32.read, hsr, 0);
         else
-            return handle_raz_wi(regs, r, cp32.read, hsr, 1);
+            return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
     case HSR_CPREG32(PMINTENSET):
     case HSR_CPREG32(PMINTENCLR):
         /* EL1 only, however MDCR_EL2.TPM==1 means EL0 may trap here also. */
-        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
     case HSR_CPREG32(PMCR):
     case HSR_CPREG32(PMCNTENSET):
     case HSR_CPREG32(PMCNTENCLR):
@@ -1763,7 +1799,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
          * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
          * emulate that register as 0 above.
          */
-        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
 
     /*
      * HCR_EL2.TIDCP
@@ -1866,7 +1902,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
 static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
 {
     const struct hsr_cp32 cp32 = hsr.cp32;
-    register_t *r = select_user_reg(regs, cp32.reg);
+    int regidx = cp32.reg;
     struct domain *d = current->domain;
 
     if ( !check_conditional_instr(regs, hsr) )
@@ -1888,9 +1924,9 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
      *    DBGPRCR
      */
     case HSR_CPREG32(DBGOSLAR):
-        return handle_wo_wi(regs, r, cp32.read, hsr, 1);
+        return handle_wo_wi(regs, regidx, cp32.read, hsr, 1);
     case HSR_CPREG32(DBGOSDLR):
-        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
 
     /*
      * MDCR_EL2.TDA
@@ -1915,6 +1951,9 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
      *    DBGOSECCR
      */
     case HSR_CPREG32(DBGDIDR):
+    {
+        uint32_t val;
+
         /*
          * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
          * is set to 0, which we emulated below.
@@ -1928,23 +1967,26 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
          *  - Version: ARMv7 v7.1
          *  - Variant and Revision bits match MDIR
          */
-        *r = (1 << 24) | (5 << 16);
-        *r |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
+        val = (1 << 24) | (5 << 16);
+        val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
+        set_user_reg(regs, regidx, val);
+
         break;
+    }
 
     case HSR_CPREG32(DBGDSCRINT):
         /*
          * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
          * is set to 0, which we emulated below.
          */
-        return handle_ro_raz(regs, r, cp32.read, hsr, 1);
+        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
 
     case HSR_CPREG32(DBGDSCREXT):
         /*
          * Implement debug status and control register as RAZ/WI.
          * The OS won't use Hardware debug if MDBGen not set.
          */
-        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
 
     case HSR_CPREG32(DBGVCR):
     case HSR_CPREG32(DBGBVR0):
@@ -1953,7 +1995,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     case HSR_CPREG32(DBGWCR0):
     case HSR_CPREG32(DBGBVR1):
     case HSR_CPREG32(DBGBCR1):
-        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
 
     /*
      * CPTR_EL2.TTA
@@ -2077,7 +2119,7 @@ static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
 static void do_sysreg(struct cpu_user_regs *regs,
                       const union hsr hsr)
 {
-    register_t *x = select_user_reg(regs, hsr.sysreg.reg);
+    int regidx = hsr.sysreg.reg;
     struct vcpu *v = current;
 
     switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
@@ -2091,7 +2133,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
         if ( psr_mode_is_user(regs) )
             return inject_undef_exception(regs, hsr);
         if ( hsr.sysreg.read )
-           *x = v->arch.actlr;
+            set_user_reg(regs, regidx, v->arch.actlr);
         break;
 
     /*
@@ -2100,7 +2142,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
      * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
      */
     case HSR_SYSREG_MDRAR_EL1:
-        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 1);
+        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);
 
     /*
      * MDCR_EL2.TDOSA
@@ -2112,9 +2154,9 @@ static void do_sysreg(struct cpu_user_regs *regs,
      *    DBGPRCR_EL1
      */
     case HSR_SYSREG_OSLAR_EL1:
-        return handle_wo_wi(regs, x, hsr.sysreg.read, hsr, 1);
+        return handle_wo_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
     case HSR_SYSREG_OSDLR_EL1:
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
 
     /*
      * MDCR_EL2.TDA
@@ -2134,18 +2176,18 @@ static void do_sysreg(struct cpu_user_regs *regs,
      *    DBGAUTHSTATUS_EL1
      */
     case HSR_SYSREG_MDSCR_EL1:
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
     case HSR_SYSREG_MDCCSR_EL0:
         /*
          * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that
          * register as RAZ/WI above. So RO at both EL0 and EL1.
          */
-        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0);
+        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0);
     HSR_SYSREG_DBG_CASES(DBGBVR):
     HSR_SYSREG_DBG_CASES(DBGBCR):
     HSR_SYSREG_DBG_CASES(DBGWVR):
     HSR_SYSREG_DBG_CASES(DBGWCR):
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
 
     /*
      * MDCR_EL2.TPM
@@ -2169,13 +2211,13 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * Accessible from EL1 only, but if EL0 trap happens handle as
          * undef.
          */
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
     case HSR_SYSREG_PMUSERENR_EL0:
         /* RO at EL0. RAZ/WI at EL1 */
         if ( psr_mode_is_user(regs) )
-            return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0);
+            return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0);
         else
-            return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+            return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
     case HSR_SYSREG_PMCR_EL0:
     case HSR_SYSREG_PMCNTENSET_EL0:
     case HSR_SYSREG_PMCNTENCLR_EL0:
@@ -2192,7 +2234,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
          * emulate that register as 0 above.
          */
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
 
     /*
      * !CNTHCTL_EL2.EL1PCEN

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index dba2449168c01755de5ce12364a48518abfb2637..b37a7c0763d2ca0e67f7084329dbb57886303183 100644 (file)
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1304,7 +1304,6 @@ static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
 {
     struct vcpu *v = current;
     struct hsr_sysreg sysreg = hsr.sysreg;
-    register_t *r = select_user_reg(regs, sysreg.reg);
 
     ASSERT (hsr.ec == HSR_EC_SYSREG);
 
@@ -1318,7 +1317,7 @@ static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
     case HSR_SYSREG_ICC_SGI1R_EL1:
         /* WO */
         if ( !sysreg.read )
-            return vgic_v3_to_sgi(v, *r);
+            return vgic_v3_to_sgi(v, get_user_reg(regs, sysreg.reg));
         else
         {
             gprintk(XENLOG_WARNING, "Reading SGI1R_EL1 - WO register\n");

diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index b9c0b41b3e2c8f2680303d23d7d8a20e44283d72..f63670564a91a86752c5d6201879c5e3d2da1b09 100644 (file)
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -249,32 +249,49 @@ static int vtimer_cntp_cval(struct cpu_user_regs *regs, uint64_t *r, int read)
 static int vtimer_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr)
 {
     struct hsr_cp32 cp32 = hsr.cp32;
-    uint32_t *r = (uint32_t *)select_user_reg(regs, cp32.reg);
+    /*
+     * Initialize to zero to avoid leaking data if there is an
+     * implementation error in the emulation (such as not correctly
+     * setting r).
+     */
+    uint32_t r = 0;
+    int res;
+
 
     if ( cp32.read )
         perfc_incr(vtimer_cp32_reads);
     else
         perfc_incr(vtimer_cp32_writes);
 
+    if ( !cp32.read )
+        r = get_user_reg(regs, cp32.reg);
+
     switch ( hsr.bits & HSR_CP32_REGS_MASK )
     {
     case HSR_CPREG32(CNTP_CTL):
-        return vtimer_cntp_ctl(regs, r, cp32.read);
+        res = vtimer_cntp_ctl(regs, &r, cp32.read);
+        break;
 
     case HSR_CPREG32(CNTP_TVAL):
-        return vtimer_cntp_tval(regs, r, cp32.read);
+        res = vtimer_cntp_tval(regs, &r, cp32.read);
+        break;
 
     default:
         return 0;
     }
+
+    if ( res && cp32.read )
+        set_user_reg(regs, cp32.reg, r);
+
+    return res;
 }
 
 static int vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
 {
     struct hsr_cp64 cp64 = hsr.cp64;
-    uint32_t *r1 = (uint32_t *)select_user_reg(regs, cp64.reg1);
-    uint32_t *r2 = (uint32_t *)select_user_reg(regs, cp64.reg2);
-    uint64_t x = (uint64_t)(*r1) | ((uint64_t)(*r2) << 32);
+    uint32_t r1 = get_user_reg(regs, cp64.reg1);
+    uint32_t r2 = get_user_reg(regs, cp64.reg2);
+    uint64_t x = (uint64_t)r1 | ((uint64_t)r2 << 32);
 
     if ( cp64.read )
         perfc_incr(vtimer_cp64_reads);
@@ -294,8 +311,8 @@ static int vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
 
     if ( cp64.read )
     {
-        *r1 = (uint32_t)(x & 0xffffffff);
-        *r2 = (uint32_t)(x >> 32);
+        set_user_reg(regs, cp64.reg1, x & 0xffffffff);
+        set_user_reg(regs, cp64.reg2, x >> 32);
     }
 
     return 1;
@@ -311,14 +328,16 @@ static int vtimer_emulate_sysreg32(struct cpu_user_regs *regs, union hsr hsr,
                                    vtimer_sysreg32_fn_t fn)
 {
     struct hsr_sysreg sysreg = hsr.sysreg;
-    register_t *x = select_user_reg(regs, sysreg.reg);
-    uint32_t r = *x;
+    uint32_t r = 0;
     int ret;
 
+    if ( !sysreg.read )
+        r = get_user_reg(regs, sysreg.reg);
+
     ret = fn(regs, &r, sysreg.read);
 
     if ( ret && sysreg.read )
-        *x = r;
+        set_user_reg(regs, sysreg.reg, r);
 
     return ret;
 }
@@ -327,9 +346,23 @@ static int vtimer_emulate_sysreg64(struct cpu_user_regs *regs, union hsr hsr,
                                    vtimer_sysreg64_fn_t fn)
 {
     struct hsr_sysreg sysreg = hsr.sysreg;
-    uint64_t *x = select_user_reg(regs, sysreg.reg);
+    /*
+     * Initialize to zero to avoid leaking data if there is an
+     * implementation error in the emulation (such as not correctly
+     * setting x).
+     */
+    uint64_t x = 0;
+    int ret;
+
+    if ( !sysreg.read )
+        x = get_user_reg(regs, sysreg.reg);
 
-    return fn(regs, x, sysreg.read);
+    ret = fn(regs, &x, sysreg.read);
+
+    if ( ret && sysreg.read )
+        set_user_reg(regs, sysreg.reg, x);
+
+    return ret;
 }
 
 static int vtimer_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)

diff --git a/xen/include/asm-arm/regs.h b/xen/include/asm-arm/regs.h
index 56d53d65cf691131dc13d9f285096cecc49f2c68..2440edb29ae9a7deb223397fd5d0aef7fd5f3e39 100644 (file)
--- a/xen/include/asm-arm/regs.h
+++ b/xen/include/asm-arm/regs.h
@@ -50,11 +50,8 @@
 
 #define return_reg(v) ((v)->arch.cpu_info->guest_cpu_user_regs.r0)
 
-/*
- * Returns a pointer to the given register value in regs, taking the
- * processor mode (CPSR) into account.
- */
-extern register_t *select_user_reg(struct cpu_user_regs *regs, int reg);
+register_t get_user_reg(struct cpu_user_regs *regs, int reg);
+void set_user_reg(struct cpu_user_regs *regs, int reg, register_t val);
 
 #endif